Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

種々のリファクタリング等のまとめ #152

Merged
merged 6 commits into from
Jan 9, 2024

Conversation

ciscorn
Copy link
Member

@ciscorn ciscorn commented Jan 6, 2024

変更内容

#153

仮のTransformerで「とりあえず」の全頂点の変換処理を行う

  • x-y 入れかえ処理を入れておく
    • これにともない GeoJSON, GeoPackage ドライバ内での x-y 入れかえは除去する
  • CRSを WGS 84 にしておく処理も追加 JGD2011→WGS 84の変換を加える #155

Gpkg, GeoJSON 周りのリファクタリング

  • nusamai-geojson に "indexed" でないふつうのジオメトリをGeoJSON Valueに変換する関数を追加(tiling2d sink で使う)。
  • Gpkg のバイナリジオメトリの構築で、 Vec<u8> でなく std::io::Write trait を使う(Vec<u8>はWrite)。一時的なVecの生成も除去。
  • Gpkg sink が、インメモリSQLiteを扱えていない(:memory: というファイルを作ってしまう)問題を修正

その他

@ciscorn ciscorn self-assigned this Jan 6, 2024
@ciscorn ciscorn changed the title pipeline: monor refactoring and tests pipeline: minor refactoring and tests Jan 6, 2024
Copy link

codecov bot commented Jan 6, 2024

Codecov Report

Attention: 24 lines in your changes are missing coverage. Please review.

Files Patch % Lines
nusamai-gpkg/src/handler.rs 50.00% 6 Missing ⚠️
nusamai/src/sink/gpkg/mod.rs 66.66% 6 Missing ⚠️
nusamai-geojson/src/conversion.rs 98.07% 3 Missing ⚠️
nusamai/src/pipeline/feedback.rs 0.00% 3 Missing ⚠️
nusamai/src/sink/geojson/mod.rs 66.66% 2 Missing ⚠️
nusamai/src/main.rs 0.00% 1 Missing ⚠️
nusamai/src/pipeline/runner.rs 66.66% 1 Missing ⚠️
nusamai/src/source/citygml.rs 85.71% 1 Missing ⚠️
nusamai/src/transform/mod.rs 96.00% 1 Missing ⚠️
Additional details and impacted files
Components Coverage Δ
GUI ∅ <ø> (∅)
Backend 76.10% <77.61%> (+34.64%) ⬆️
Libraries 86.32% <96.89%> (+0.65%) ⬆️

📢 Thoughts on this report? Let us know!

@ciscorn ciscorn force-pushed the minor-refactor-pipeline branch 3 times, most recently from e91f8cb to 5dc0bb2 Compare January 6, 2024 03:14
@ciscorn ciscorn force-pushed the minor-refactor-pipeline branch from 5dc0bb2 to 292d00e Compare January 6, 2024 03:36
@ciscorn ciscorn requested a review from a team January 6, 2024 04:08
@ciscorn ciscorn marked this pull request as ready for review January 6, 2024 04:09
@ciscorn ciscorn changed the title pipeline: minor refactoring and tests pipeline: Minor fixes and tests Jan 6, 2024
@ciscorn ciscorn force-pushed the minor-refactor-pipeline branch 2 times, most recently from 272a0d8 to 6bf4059 Compare January 6, 2024 12:26
@ciscorn ciscorn force-pushed the minor-refactor-pipeline branch from 6bf4059 to 63874e9 Compare January 6, 2024 12:28
## 変更内容

### 仮のTransformerで「とりあえず」の変換処理を行う
- x-y 入れかえ処理を入れておく
    - これにともない GeoJSON, GeoPackage ドライバ内での x-y 入れかえは除去する
- WGS 84 にしておく処理も追加 #155

### Gpkg, GeoJSON 周りのリファクタリング
- `nusamai-geojson` に "indexed" でないふつうのジオメトリをGeoJSON
Valueに変換する関数を追加(tiling2d sink で使う)。
    - #154 もこのPRにマージ
- Gpkg のバイナリジオメトリの構築で、 `Vec<u8>` でなく `std::io::Write` trait
を使う(`Vec<u8>`はWrite)。一時的なVecの生成も除去。

---

Closes: #125
Closes: #76
@ciscorn ciscorn changed the title pipeline: Minor fixes and tests 仮のtransformer (x-y swap, to wgs84)、gpkg/geojsonのリファクタリング、その他 Jan 8, 2024
HashMap, IndexMap, etc. を多用する場面で、 Hasher を ahash に切り替える。

### 背景

Rust の HashMap / HashSet はデフォルトで SipHash-1-3 (`std::hash::SipHash`)
というハッシュ関数を使う。SipHashはHashDos攻撃への耐性が高いためにデフォルトのHasherとして採用されている。しかし高い
HashDos 耐性が必要なく、hashを多用する場面ではより軽量なHasherに切り替えることができる(切り替えることがしばしば行われる)。

- [ahash](https://crates.io/crates/ahash) - それなりの HashDos
耐性があり、パフォーマンスもよい
- [rust-lang/hashbrown](https://crates.io/crates/hashbrown)
(Rustの現在のHashMap実装の元になった crate) のデフォルトのHasherは aHash
なので、Hasherを切り替える代わりに、これを HashMap の置き換えとして使える。
- [fxhash](https://crates.io/crates/fxhash)
- [rust-lang/rustc-hash](https://crates.io/crates/rustc-hash) - rustc
で使われている Hasher
The Rust Performance Book によると、「hot iterators での Iterator::chain
の使用は可能なら避けるとよい」とのことなので、除去しておく。

https://nnethercote.github.io/perf-book/iterators.html#chaining

>
[chain](https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.chain)
can be very convenient, but it can also be slower than a single
iterator. It may be worth avoiding for hot iterators, if possible.
Copy link
Collaborator

@nokonoko1203 nokonoko1203 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!
Transformerやgeometry周り確認しつつ、動作確認もしました!

% time cd nusamai && cargo run --release -- ~/Downloads/plateau/22203_numazu-shi_2021_citygml_4_op/udx/bldg/*.gml --sink=gpkg --output=/Users/satoru/Downloads/output/output.gpkg

cargo run --release --  --sink=gpkg   11.10s user 4.44s system 358% cpu 4.334 total

@ciscorn ciscorn merged commit d4b184c into main Jan 9, 2024
4 checks passed
@ciscorn ciscorn deleted the minor-refactor-pipeline branch January 9, 2024 02:33
@ciscorn ciscorn changed the title 仮のtransformer (x-y swap, to wgs84)、gpkg/geojsonのリファクタリング、その他 種々のリファクタリング等のまとめ Jan 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants